Repository navigation
fix(marketplace): record the commit an annotated tag points to - #3161
Rodion Kazennov (nefayran) wants to merge 8 commits into
Conversation
`git ls-remote` lists an annotated or signed tag twice: the tag object
under `refs/tags/<name>` and its commit under `refs/tags/<name>^{}`.
`_parse_ls_remote_output` dropped the `^{}` line, so `apm pack` wrote the
tag object as `source.sha`, and installers that check out the tag and
compare the result rejected the pin. The tag now takes the peeled SHA, as
`parse_ls_remote_output` in `deps/git_remote_ops.py` already does.
Lightweight tags and branches keep their only SHA.
Fixes microsoft#3048
96c2f84 to
8debe84
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Peeled-tag interpretation duplicates an existing durable decision instead of routing through one canonical owner.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Fixes marketplace packing so annotated and signed tags record the checked-out commit SHA rather than the tag-object SHA.
Changes:
- Handles peeled
^{}tag references. - Adds parser and local-repository regression tests.
- Documents the fix in the changelog.
| File | Description |
|---|---|
src/apm_cli/marketplace/ref_resolver.py |
Applies peeled commit SHAs to tags. |
tests/unit/marketplace/test_ref_resolver.py |
Tests parser edge cases. |
tests/unit/marketplace/test_annotated_tag_source_sha.py |
Tests real tags and packed output. |
CHANGELOG.md |
Records the fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The marketplace parser had its own reading of peeled `^{}` records next
to the one in `deps/git_remote_ops.py`. `tag_commit_shas` in
`git_remote_ops.py` now decides which commit a tag record names, with
the annotated-tag security note moved along; `parse_ls_remote_output`
and the marketplace `_parse_ls_remote_output` both read tags through
it, and the marketplace module no longer mentions `^{}`.
The owner is registered as `ls-remote-tag-commits` with the guard
`transport-platform-ls-remote-tag-commits`: one definition of
`tag_commit_shas`, and no `"^{}"` literal in `src/apm_cli` outside the
owner. The guard has its mutation case, and `tag_commit_shas` has
direct unit tests.
f8859f2 to
efbfe64
Compare
Preserve contributor history and validate microsoft#3161 against current main 9fb5ce8. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
APM Review Panel:
|
| PR | head SHA | CEO stance | iters | folds | defers | Copilot rounds | CI | mergeable | mergeStateStatus | notes |
|---|---|---|---|---|---|---|---|---|---|---|
| #3161 | ba052812c2 |
ship_now | 4 | 4 | 1 | 2 | green | MERGEABLE | BLOCKED | Existing reviewer requests retained; no merge performed |
Convergence
4 iterations; two Copilot rounds; no unresolved in-scope finding. Existing author and reviewer ownership is unchanged. Ready for maintainer review.
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
Generated by autopilot-pr-merge-worker. This comment is AI-generated and may contain errors.
Address the microsoft#3161 panel follow-ups with a real-git exact-SHA fixture, one peeled-record predicate, explicit marketplace iteration intent, and concise producer recovery guidance. Preserve selected refs and contributor history. Mutation trials fail when peeled identity, its static guard, or the exact-pin short circuit is removed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Address the exact-head Spec conformance failure with executable existing req-rs-002 and req-rs-017 coverage. Both record orders must preserve the peeled commit and annotated-only candidate selection. The mutation loses annotated identity and both cases fail. No new normative requirement or waiver. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Refresh microsoft#3161 for independently landed microsoft#3222 and earlier microsoft#3100 before final exact-head lint and functional validation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Preserve microsoft#3237 linked-resource lifetime clarification alongside microsoft#3222 and microsoft#3100. No implementation or scope change to microsoft#3161. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Description
git ls-remotelists an annotated or signed tag twice: the tag object underrefs/tags/<name>and the commit it points to underrefs/tags/<name>^{}._parse_ls_remote_outputinsrc/apm_cli/marketplace/ref_resolver.pydropped the^{}line, soapm packwrote the tag object assource.sha. A checkout of the tag lands on the commit, and installers that compare the two reject the pin, as in thecavemancase in the issue.Both parsers now read tags through
tag_commit_shasinsrc/apm_cli/deps/git_remote_ops.py, which gives a tag the SHA from its^{}line; that line adds no ref of its own. It is registered as the ownerls-remote-tag-commits, with the static guardtransport-platform-ls-remote-tag-commits. Lightweight tags and branches keep their only SHA. Aref:that is a full SHA never reaches this parser, so exact pins are unchanged.resolve_ref_shauses the marketplace parser and is only called withHEAD, which has no^{}line.Issue and approved scope
Issue: #3048
Human scope-approval comment: #3048 (comment)
This PR completes the issue. A
marketplace.jsonpacked before the fix keeps the tag object SHA untilapm packruns again.Type of change
Testing
tests/unit/marketplace/test_ref_resolver.py:test_peeled_tag_skippedis nowtest_annotated_tag_takes_peeled_commit_shaand expects the commit SHA. New cases cover a^{}line that comes before its tag line, a branch with a lightweight and an annotated tag, and a^{}line with no tag line.tests/unit/marketplace/test_annotated_tag_source_sha.pycreates the tags with git in a local bare repository (LocalGitRepositoryFactory): annotated, lightweight, and SSH-signed (skipped whenssh-keygenis missing). It packs a marketplace withref: v1.0.0,version: "~1.0.0"andref: v1.1.0entries, then clones the repository, checks out eachsource.refand comparesHEADwithsource.sha, the same check an installer makes. With the old parser, six of the new tests fail.tests/unit/deps/test_git_remote_ops.pycoverstag_commit_shasdirectly, and the guard has its case intests/integration/test_architecture_owner_rule_mutations.py. On 20,000 randomls-remoteoutputsparse_ls_remote_outputreturns the same refs before and after the change.Locally
uv run pytest tests/unit tests/test_console.pypasses (23585 passed, 42 skipped, 21 xfailed). Ruff check and format, the pylint R0801 duplication check,scripts/lint-auth-signals.shandscripts/lint-architecture-boundaries.share clean.Spec conformance (OpenAPM v0.1)
docs/src/content/docs/specs/openapm-v0.1.mdupdated(new/changed
<a id="req-XXX"></a>anchor + prose + Appendix Crow).
docs/src/content/docs/specs/manifests/openapm-v0.1.requirements.ymlupdated.
@pytest.mark.req("req-XXX")test undertests/spec_conformance/added or extended.CONFORMANCE.{md,json}regenerated viauv run --extra dev python -m tests.spec_conformance.gen_statementand committed.